This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Greptile Summary

This PR implements Empty Block Complementing for Slurm block topology: when accelerator domains have more hosts than baseBlockSize, or when domains are unevenly sized, the emitted block list is padded with structural empty-block placeholders so Slurm can reason about the full tree capacity. The feature is activated on both the cluster-wide Slurm block path and the per-partition YAML path.

• block_pack.go / block_complement.go: New files that build a partition-scoped ordered domain list, split oversized domains across multiple base blocks, pad each domain group to a power-of-two multiple, and expand the list to the next tree-capacity boundary. hostsForBlock intentionally filters to only the partition's own nodes, preventing cross-partition node leakage previously flagged in review.
• block.go: After complement, refreshes nodeInfo.blockID so GetNodeTopologySpec always returns IDs that match the emitted topology file — fixes the stale-ID bug noted in earlier review threads.
• slurm.go: Adds validateBlockSizes (called eagerly for both global and per-partition configs) and changes GetTranslateConfig to return *httperr.Error directly, avoiding double-wrapping and enabling a 400 Bad Request for malformed inputs.

Confidence Score: 5/5

Safe to merge. The complement logic is well-guarded, partition-scoped, and deterministic; all previously flagged issues have been addressed.

All three substantive issues from prior review rounds (stale blockID, nil dereference in initBlocks, cross-partition node contamination) are explicitly fixed and regression-tested. The new packing/complement code is thoroughly unit-tested across edge cases. The only remaining nit is a misleading error message wording in validateBlockSizes.

pkg/engines/slurm/slurm.go — the validateBlockSizes error messages are slightly inaccurate in their wording but do not affect correctness.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[toBlockTopology / getBlockTopologyUnit] --> B[complementBlocks]
    B --> C{fanoutsPerLevel OK\n& domains != nil?}
    C -- No --> Z[return original blocks]
    C -- Yes --> D[orderedDomainsForBlocks\nfilter to partition-local nodes]
    D --> E{all blocks have\ndomain entries?}
    E -- No --> Z
    E -- Yes --> F[groupSizeFromOrderedDomains\nsmallest 2^n x base ge maxAccelSize]
    F --> G[packOrderedDomainsIntoBlocks\nsplit + pad each domain group]
    G --> H{len out != len blocks?}
    H -- Yes --> I[expandedBaseBlockSlots\npad to next tree capacity]
    H -- No --> J[assignSequentialBlockIDs]
    I --> J
    J --> K{shouldUseComplementedBlocks?}
    K -- hasEmptySlots or len>input --> L[return complemented blocks]
    K -- No --> Z
    L --> M[refresh nodeInfo.blockID\nfor GetNodeTopologySpec]

_{Reviews (17): Last reviewed commit: "replace the temporary block tree used fo..." | Re-trigger Greptile}

Codecov Report

❌ Patch coverage is 78.77358% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.08%. Comparing base (1875ab8) to head (f59beef).
⚠️ Report is 69 commits behind head on main.

@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
+ Coverage   68.46%   71.08%   +2.62%     
==========================================
  Files          82       86       +4     
  Lines        4842     5219     +377     
==========================================
+ Hits         3315     3710     +395     
+ Misses       1395     1318      -77     
- Partials      132      191      +59     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.